Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use file picker component directly #3995

Merged
merged 5 commits into from
Nov 30, 2024
Merged

fix: use file picker component directly #3995

merged 5 commits into from
Nov 30, 2024

Conversation

elzody
Copy link
Contributor

@elzody elzody commented Sep 6, 2024

Summary

The previous implementation stopped working because the file picker's container prop expects a selector, but an element was passed instead. This PR fixes that, and uses the file picker component directly.

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Documentation (manuals or wiki) has been updated or is not required

@elzody elzody requested a review from juliusknorr September 6, 2024 20:05
@elzody elzody self-assigned this Sep 9, 2024
@juliusknorr
Copy link
Member

Could you also add a cypress test for that?

Signed-off-by: Elizabeth Danzberger <lizzy7128@tutanota.de>
@juliusknorr
Copy link
Member

@elzody took me quite a bit but found why this failed. On your local setup it likely worked because you had text enabled which dispatches an RenderReferenceEvent. This event is listened for to provide the list of providers. I left my commits with debugging attempts for you to follow along for now. Feel free to clean them up and get this merged.

After seeing that the initial state was empty if checked where in the server that was set. Then I locally set a breakpoint to see which event dispatching actually triggers the listener on my local setup. That was text, so disabling text showed the same beahavior as CI when running the cypress test.

elzody and others added 4 commits November 29, 2024 16:21
Signed-off-by: Elizabeth Danzberger <lizzy7128@tutanota.de>
Signed-off-by: Elizabeth Danzberger <lizzy7128@tutanota.de>
Signed-off-by: Elizabeth Danzberger <lizzy7128@tutanota.de>
Signed-off-by: Julius Knorr <jus@bitgrid.net>
@nextcloud nextcloud deleted a comment from cypress bot Nov 29, 2024
@elzody
Copy link
Contributor Author

elzody commented Nov 29, 2024

@juliusknorr Thanks for the help, seems like it required some really deep debugging. I think I managed to clean up the nonsense debug commits, and it seems like everything passes now!

@juliusknorr juliusknorr merged commit 0831289 into main Nov 30, 2024
73 checks passed
@juliusknorr juliusknorr deleted the fix/3957 branch November 30, 2024 07:50
@juliusknorr juliusknorr added the bug Something isn't working label Dec 2, 2024
@juliusknorr
Copy link
Member

@elzody Can you check if and how far we need to backport this one?

@elzody
Copy link
Contributor Author

elzody commented Dec 2, 2024

@juliusknorr Yep, only back to stable30. I already have a PR open for it: #4028

I just need to replicate what you added here and get it cleaned up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-request bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Smart picker not working for document sections
2 participants